Skip to content

[GreenDonut] Properly support ToBatchPageAsync with valueSelector#9324

Merged
michaelstaib merged 8 commits intomainfrom
tte/fix-batch-page-with-value-selector
Mar 21, 2026
Merged

[GreenDonut] Properly support ToBatchPageAsync with valueSelector#9324
michaelstaib merged 8 commits intomainfrom
tte/fix-batch-page-with-value-selector

Conversation

@tobias-tengler
Copy link
Copy Markdown
Member

@tobias-tengler tobias-tengler commented Mar 8, 2026

No description provided.

@github-actions github-actions Bot added 📚 documentation This issue is about working on our documentation. 🌶️ hot chocolate 🌶️ green donut labels Mar 8, 2026
@tobias-tengler tobias-tengler force-pushed the tte/fix-batch-page-with-value-selector branch from eda626f to 42593ca Compare March 8, 2026 10:42
@tobias-tengler tobias-tengler marked this pull request as ready for review March 8, 2026 10:43
Copilot AI review requested due to automatic review settings March 8, 2026 10:43
Comment thread src/GreenDonut/src/GreenDonut.Data.Primitives/ElementCursorPage.cs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a bug in the GreenDonut paging framework where ToBatchPageAsync with a valueSelector did not properly generate cursors for the projected items, because the cursor needed to be created from the source element rather than the projected value. The fix introduces an ElementCursorPage<TElement, TValue> concept that retains the mapping between original elements and projected items for cursor creation.

Changes:

  • Page<T> is refactored from sealed to abstract, with two new internal concrete implementations: ValueCursorPage<T> (for direct item→cursor mapping) and ElementCursorPage<TElement, TValue> (for source element→cursor mapping when a value selector is used).
  • ToBatchPageAsync is updated to accept a nullable valueSelector, using ElementCursorPage when a selector is provided to preserve the source→cursor relationship.
  • Migration documentation and tests are updated to reflect the API change from new Page<T>(...) to Page<T>.Create(...).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
GreenDonut.Data.Primitives/Page.cs Changed from sealed to abstract; added Page<T>.Create(...) factory methods delegating to ValueCursorPage<T> and ElementCursorPage<TElement,T>
GreenDonut.Data.Primitives/ValueCursorPage.cs New internal class for pages where cursors are created directly from items
GreenDonut.Data.Primitives/ElementCursorPage.cs New internal class for pages where cursors are created from source elements (supports valueSelector use case)
GreenDonut.Data.EntityFramework/Extensions/PagingQueryableExtensions.cs Updated ToBatchPageAsync to support nullable valueSelector, uses ElementCursorPage when selector is provided; extracted CreatePageFlags helper
HotChocolate/Data/test/Data.Tests/PagingHelperIntegrationTests.cs Added test for ToBatchPageAsync with valueSelector and ToConnectionAsync; added SeedMinimalAsync helper
HotChocolate/Data/test/Data.Tests/QueryContextUnionProjectionTests.cs Updated to use new Page<T>.Create(...) factory method instead of removed public constructor
website/src/docs/hotchocolate/v16/migrating/migrate-from-15-to-16.md Added migration guidance for the Page<T> constructor-to-factory-method change

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/GreenDonut/src/GreenDonut.Data.Primitives/ElementCursorPage.cs Outdated
Comment thread src/GreenDonut/src/GreenDonut.Data.Primitives/ElementCursorPage.cs Outdated
@tobias-tengler tobias-tengler added this to the HC-16.0.0 milestone Mar 9, 2026
@tobias-tengler tobias-tengler force-pushed the tte/fix-batch-page-with-value-selector branch from 42593ca to 33d8a42 Compare March 9, 2026 09:54
tobias-tengler and others added 4 commits March 19, 2026 08:55
… notes

- Guard ToBatchPageAsync against null valueSelector when TElement != TValue
  to surface ArgumentNullException instead of InvalidCastException
- Add backward pagination test (Last=2) with valueSelector and ToConnectionAsync
- Expand v15→v16 migration guide with all Page/cursor/Edge API changes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@michaelstaib michaelstaib force-pushed the tte/fix-batch-page-with-value-selector branch from 904b03f to c1925bf Compare March 19, 2026 08:56
@michaelstaib michaelstaib self-assigned this Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📚 documentation This issue is about working on our documentation. 🌶️ green donut 🌶️ hot chocolate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants